Harden program-cache permissions, binary-path resolution, and coredump helper lifetime by rparolin · Pull Request #2399 · NVIDIA/cuda-python

rparolin · 2026-07-21T19:42:54Z

Small safety fixes across three areas, each with a test. All hardening — no intentional behavior change.

What changed

cuda.core

The program cache writes compiled GPU code to disk. The tmp/ staging area (where in-flight writes land before the atomic rename into entries/) is now created owner-only (0o700) so other local users can't read or replace device code mid-write. Per review, root/ and entries/ intentionally inherit the umask / any pre-existing permissions so a deliberately shared kernel cache (e.g. group-writable on a compute cluster) keeps working — an existing directory is used as-is, never re-tightened.

cuda.pathfinder

find_nvidia_binary_utility now always returns a full absolute path, as the docs promise (it could return a relative one before).
On Windows, if AddDllDirectory fails, we now warn instead of failing silently.

cuda.bindings

Fix two memory bugs in the coredump-settings helper (_HelperCUcoredumpSettings): it kept a pointer to bytes it didn't own (dangling pointer), and never freed a 1 KB buffer (leak).

Security scope

This started as "make the whole cache tree owner-only regardless of umask." Per review it was narrowed to keep shared caches working, so the residual posture is:

Confidentiality — closed in practice. Committed entries are written via mkstemp (0o600) and os.replace preserves that mode, so cached kernel contents stay owner-readable even though entries/ inherits the umask. Other local users can list entry filenames (key hashes) but can't read the code.
Tampering on a normal single-user box — closed. Only the owner can write entries/, so no one can plant a kernel this process later loads.
Tampering in a deliberately shared (group-writable) cache — accepted trade-off, NOT closed. A group member with write access to entries/ could replace a cached kernel. Tamper-proofing that path needs load-time integrity checks (a separate change), so it's out of scope here. Flagging for sign-off.

Tests

Added tests for the tmp/ permissions (and that a shared root is preserved), the absolute path, and the coredump helper. Directory-permission logic verified locally; full suite on RTX 5880.

Reviewers: @leofang (core/bindings), @rwgk (pathfinder).

…er lifetime - cuda.core: create the on-disk program cache tree owner-only (0o700) and re-assert restrictive perms on POSIX, so cached device code cannot be read or planted by other local users regardless of the inherited umask. - cuda.pathfinder: absolutize the resolved binary-utility path to honor the documented absolute-path contract; surface AddDllDirectory failures on Windows with GetLastError instead of silently swallowing them. - cuda.bindings: retain the caller's bytes in _HelperCUcoredumpSettings so the borrowed pointer cannot outlive its backing buffer, and free the getter's 1 KiB buffer in __dealloc__; clarify get_buffer_pointer's lifetime contract. Adds regression tests for the cache permissions, path absolutization, and coredump helper lifetime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

copy-pr-bot · 2026-07-21T19:42:57Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Shorten the explanatory comments to a line or two each and drop the internal issue-number references (which don't resolve in this repo). No code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

It didn't actually guard the two lifetime fixes: the driver copies the path during the set call (so the NVIDIA#379 latent UAF can't trigger), and a missing free leaks silently (so NVIDIA#381 wouldn't fail the test either). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The test pre-creates a 0o777 cache dir to verify the loader tightens it to 0o700; the permissive mask is the point of the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The abspath change makes find_nvidia_binary_utility return a drive-qualified path on Windows (C:\... ), so the three search-order tests must compare against os.path.abspath(expected). No-op on Linux; fixes the Windows CI failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ting The c_int/c_byte param feeders silently narrowed an out-of-range Python int (e.g. 2**32+5 -> 5). Range-check in the feeder and raise OverflowError; declare feed() as 'except? -1' so Cython propagates the exception instead of swallowing it. Addresses Glasswing V9.1 (leofang chose the raise option over matching ctypes' silent wrap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Per Leo's review: on 3.13+ PyLong_AsInt converts and range-checks in one call (raising OverflowError itself), so gate the manual INT_MIN/INT_MAX check to pre-3.13 only. c_byte keeps the manual check (no 8-bit CPython equivalent). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…flow Per Leo's follow-up: use a single code path on all supported Pythons instead of version-gating PyLong_AsInt. PyLong_AsLongAndOverflow flags out-of-long values via its overflow out-param (no exception set), then we bounds-check the 32-bit (c_int) / 8-bit (c_byte) target range and raise OverflowError. Drops the PY_VERSION_HEX gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…#2402) The out-of-range c_int/c_byte -> OverflowError change is a distinct behavior change; moved to a standalone PR so it can be reviewed separately from this hardening batch. No functional change to the remaining hardening work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@leofang

Per @leofang's review on PR NVIDIA#2399: - _file_stream.py: only the tmp/ staging dir is created owner-only (0o700). root/ and entries/ now inherit the umask / any pre-existing permissions so a deliberately shared kernel cache (e.g. group-writable on a compute cluster) keeps working. Dropped the post-mkdir chmod that would have re-tightened an existing shared directory. 0o700 in mkdir mode needs no chmod: umask only clears bits. - _internal/utils.pyx: revert the get_buffer_pointer docstring change; that is cybind code and is out of scope for this PR. - Tests updated to match: assert only tmp is 0o700, and that a pre-existing 0o777 shared root is used as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Record the residual risk from narrowing the cache-permission hardening (glasswing NVIDIA#359/NVIDIA#375) per PR NVIDIA#2399 review: committed entry files stay 0o600 (contents owner-only via mkstemp+os.replace), tmp/ is owner-only to protect in-flight writes, but root/entries inherit the umask so shared caches keep working. In a group-writable shared cache a group member could replace a cached kernel; tamper-proofing that path needs load-time integrity checks and is out of scope here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rparolin · 2026-07-22T17:15:53Z

Heads-up on the security posture after narrowing the cache-permission fix (details in the updated PR description under Security scope):

Cached kernel contents stay owner-only (0o600 via mkstemp+os.replace), and tmp/ is owner-only, so confidentiality and in-flight tampering are covered.
On a normal single-user box, planting a kernel into entries/ isn't possible either.
In a deliberately shared, group-writable cache, a group member could replace a cached entry that this process later loads. That's the accepted trade-off for keeping shared caches working — permissions alone can't both share-write and lock out writers; closing it would need load-time integrity checks (separate change).

@leofang flagging for sign-off on accepting that residual risk (mitigated for single-user; shared-cache tampering accepted).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rparolin · 2026-07-22T17:20:44Z

Ready for another look — pushed changes addressing the review:

@leofang (cuda.core / cuda.bindings)

Cache permissions narrowed to your guidance: only tmp/ is created owner-only (0o700); root/ and entries/ inherit the umask / any pre-existing perms, so a deliberately shared cache is used as-is and never re-tightened. Dropped the post-mkdir chmod.
Reverted the get_buffer_pointer docstring change in _internal/utils.pyx — that's cybind, left untouched.
The _HelperCUcoredumpSettings dangling-pointer + 1 KB leak fix is unchanged.
Please also sign off on the documented residual risk under Security scope (shared group-writable cache tampering — accepted trade-off).

@rwgk (cuda.pathfinder)

find_nvidia_binary_utility now always returns an absolute path (honors the documented contract).
Windows AddDllDirectory failures now warn instead of being swallowed.

Thanks!

leofang · 2026-07-22T21:21:02Z

cuda.core LGTM!

@leofang

Per @leofang's review question: replace the direct ctypes kernel32.AddDllDirectory() call with the stdlib os.add_dll_directory() wrapper. Both call the same Win32 API, but the stdlib version drops the hand-maintained argtypes/restype binding and reports failure as OSError instead of a NULL cookie + GetLastError() check. Behavior is unchanged: the returned handle is intentionally discarded (it has no finalizer, so the directory stays on the search path for the process lifetime), and the PATH mutation + failure warning are preserved. Also strip an internal issue number from a test docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mdboom

LGTM

mdboom · 2026-07-23T13:31:16Z

-            return candidate
+            # Return an absolute path, as the docstring promises (a relative
+            # search dir would otherwise leak a relative result).
+            return os.path.abspath(candidate)


We really should migrate from os.path to pathlib.Path (which has far fewer foot-guns). #2410

@mdboom Can you create a ticket for that work? We can then prioritize it and remove it from the code base completely. Should we also include a pre-commmit check that prevents people from creating new code with os.path in it?

github-actions · 2026-07-23T16:31:15Z

Doc Preview CI
Preview removed because the pull request was closed or merged.

rparolin added this to the cuda.core next milestone Jul 21, 2026

rparolin added bug Something isn't working cuda.bindings Everything related to the cuda.bindings module cuda.core Everything related to the cuda.core module cuda.pathfinder Everything related to the cuda.pathfinder module labels Jul 21, 2026

rparolin requested review from leofang and rwgk July 21, 2026 20:40

rparolin marked this pull request as ready for review July 21, 2026 20:41

rparolin and others added 3 commits July 21, 2026 13:47

Simplify code comments on the hardening fixes

a243030

Shorten the explanatory comments to a line or two each and drop the internal issue-number references (which don't resolve in this repo). No code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Remove agent_authored markers from the new tests

c9e5d5d

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rparolin self-assigned this Jul 21, 2026

Silence bandit S103 on the intentionally world-writable test dir

dc8bb9f

The test pre-creates a 0o777 cache dir to verify the loader tightens it to 0o700; the permissive mask is the point of the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

This comment has been minimized.

Sign in to view

rparolin and others added 4 commits July 21, 2026 15:34

rparolin mentioned this pull request Jul 21, 2026

Raise OverflowError on out-of-range c_int/c_byte kernel arguments #2402

Open

leofang reviewed Jul 22, 2026

View reviewed changes

Comment thread cuda_bindings/cuda/bindings/_internal/utils.pyx Outdated

leofang reviewed Jul 22, 2026

View reviewed changes

Comment thread cuda_core/cuda/core/utils/_program_cache/_file_stream.py Outdated

rparolin and others added 2 commits July 22, 2026 08:58

rparolin and others added 2 commits July 22, 2026 10:18

Simplify cache permission comment; drop internal tool name

644ae82

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Strip internal issue numbers from cache permission test docstring

8623e74

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rparolin requested a review from leofang July 22, 2026 17:20

leofang reviewed Jul 22, 2026

View reviewed changes

Comment thread cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_windows.py

rparolin requested a review from leofang July 22, 2026 22:20

mdboom approved these changes Jul 23, 2026

View reviewed changes

rparolin merged commit 8beb6e2 into NVIDIA:main Jul 23, 2026
103 of 105 checks passed

rparolin modified the milestones: cuda.core next, cuda.core 1.2.0 Jul 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Harden program-cache permissions, binary-path resolution, and coredump helper lifetime#2399

Harden program-cache permissions, binary-path resolution, and coredump helper lifetime#2399
rparolin merged 15 commits into
NVIDIA:mainfrom
rparolin:hardening/cache-perms-loader-coredump

rparolin commented Jul 21, 2026 •

edited

Loading

Uh oh!

copy-pr-bot Bot commented Jul 21, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

rparolin commented Jul 22, 2026 •

edited

Loading

Uh oh!

rparolin commented Jul 22, 2026

Uh oh!

Uh oh!

leofang commented Jul 22, 2026

Uh oh!

mdboom left a comment

Uh oh!

mdboom Jul 23, 2026

Uh oh!

rparolin Jul 23, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

rparolin commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Security scope

Tests

Uh oh!

copy-pr-bot Bot commented Jul 21, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

rparolin commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rparolin commented Jul 22, 2026

Uh oh!

Uh oh!

leofang commented Jul 22, 2026

Uh oh!

mdboom left a comment

Choose a reason for hiding this comment

Uh oh!

mdboom Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

rparolin Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rparolin commented Jul 21, 2026 •

edited

Loading

rparolin commented Jul 22, 2026 •

edited

Loading